Skip to content

feat: deploy solar-ui as part of the solar chart - #743

Merged
rebEllieous merged 5 commits into
mainfrom
feature/solar-ui-in-solar-chart
Aug 17, 2026
Merged

feat: deploy solar-ui as part of the solar chart#743
rebEllieous merged 5 commits into
mainfrom
feature/solar-ui-in-solar-chart

Conversation

@rebEllieous

@rebEllieous rebEllieous commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

What

Builds and publishes the solar-ui image in CI, and adds a ui component to the solar chart so it can actually be deployed.

Relates to (odd-internal) # 62

Why

The UI is part of SolAr but had no delivery path: the image was never published, and no chart could run it

Testing

  • Built the ui target and ran the image: GET / returns the embedded index.html (200), /assets/*.js the 584 KB bündle
  • helm lint clean. helm template renders the UI objects with GHCR repo, merged pull secrets and env-injected client secret, and fails as intended when ui.enabled=true without an issuer. Default render is identical to before

Notes for reviewers

  • RBAC: new ClusterRole for the UI ServiceAccount — get/list on namespaces (the BFF enumerates with its own identity, then filters per user via SelfSubjectRulesReview), plus impersonate on users/groups only when ui.args.authMode=impersonate
  • Security default: ui.enabled=false, and the template requireds ui.oidc.issuer / ui.oidc.redirectURL. Without an issuer the BFF serves the K8s API as its own ServiceAccount to anyone who reaches the pod

Checklist

  • Tests added/updated
  • No breaking changes
  • Readable commit history (squashed and cleaned up as desired)
  • AI code review considered and comments resolved

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review available on request

  • 🔍 Trigger review

Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment @coderabbitai review to review the latest changes. For a full review, comment @coderabbitai full review.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: cd169a75-46f0-457f-b451-088c698a9145

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/solar-ui-in-solar-chart

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cbrgm cbrgm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only nitpicks e.g. two tiny things beyond the inline suggestions

  • ui.args.authMode is the only key under args, while extraArgs sits right next to it as a sibling. Reads a bit odd, you'd maybe expect ui.authMode directly, or extraArgs nested under args. Pure naming, not important.
  • ui.imagePullSecrets doc is terser than the siblings (renderer spells out the [{name: my-secret}] shape). Could mirror that one-liner so the value docs stay uniform, up to you 🤷

Comment thread charts/solar/values.yaml Outdated
Comment thread charts/solar/values.yaml Outdated
@cbrgm

cbrgm commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Otherwise LGTM

@rebEllieous

Copy link
Copy Markdown
Contributor Author

Only nitpicks e.g. two tiny things beyond the inline suggestions

* `ui.args.authMode` is the only key under `args`, while `extraArgs` sits right next to it as a sibling. Reads a bit odd, you'd maybe expect `ui.authMode` directly, or `extraArgs` nested under `args`. Pure naming, not important.

* `ui.imagePullSecrets` doc is terser than the siblings (renderer spells out the `[{name: my-secret}]` shape). Could mirror that one-liner so the value docs stay uniform, up to you 🤷

good point, i looked into it: both these point would make the ui section more inconsistent with the rest of the file:
the args, exraArgs split is convention, applied that way in other sections of the chart as well. This also mirrors the python *args, **kwargs pattern (which is why its often used elsewhere) so at least to me not splitting it would read a bit odd to me 😅
as for having it under ui.authMode: the args block, even if it only holds a single item (for now) is what's passed onto the cli call as args for the bff. THe block serves a purpose in that way, i'd prefer to keep it as is.

as for the pullSecrets: renderer is actually the outlier, ui is consistent with apiserver and controller. Renderer looks slightly different because the pod runs in the rt's namespace, so the secret has to exist in multiple namespaces

cbrgm
cbrgm previously approved these changes Aug 12, 2026
@rebEllieous
rebEllieous merged commit 3d1ac1e into main Aug 17, 2026
21 checks passed
@rebEllieous
rebEllieous deleted the feature/solar-ui-in-solar-chart branch August 17, 2026 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants